Skip to content

Ignore visibility method, attr definition, module_function within block #1595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
tompng merged 2 commits into from
Feb 9, 2026
Merged

Ignore visibility method, attr definition, module_function within block #1595

tompng merged 2 commits into from
Feb 9, 2026
+127 −20

Conversation

@tompng
Copy link
Member

@tompng tompng commented Feb 4, 2026 •
edited
Loading

We need to ignore these within Module.new do end and any other block because it might be a metaprogramming block.

Ignoring def include extend in a block is already implemented. This pull request also make RDoc ignore visibility methods, attr definition and module_function in a block.

class A
  def f; end
  X = 1
  anonymous_module = Module.new do
    # These visibility change and attribute defition should be applied to anonymous module, but applied to A
    module_function :f
    private :f
    private_constant :X
    attr_accessor :rw
  end
end

Found while generating document in rails/rails
https://github.com/rails/rails/blob/45ee3bf84f20af55b8619b5dcc22a5e22a4ac0e7/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb#L307

module ColumnMethods
  class_methods do
    private
    def m; end # This is a private method of unknown class/module
  end

  def g; end # This method should be public, but was treated as private and not documented
end

@tompng tompng requested a deployment to fork-preview-protection February 4, 2026 19:07 — with GitHub Actions Waiting
@tompng tompng force-pushed the ignore_within_block branch from 3231430 to de00323 Compare February 4, 2026 19:08
@tompng tompng temporarily deployed to fork-preview-protection February 4, 2026 19:08 — with GitHub Actions Inactive
@matzbot
Copy link
Collaborator

matzbot commented Feb 4, 2026 •
edited
Loading

🚀 Preview deployment available at: https://a550c35f.rdoc-6cd.pages.dev (commit: 96f88d1)

@tompng tompng mentioned this pull request Feb 5, 2026
Open
st0012
st0012 reviewed Feb 8, 2026
View reviewed changes
lib/rdoc/parser/prism_ruby.rb
end

def _visit_call_alias_method(call_node)
return if @scanner.in_proc_block
Copy link
Member

@st0012 st0012 Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the same check be added to visit_alias_method_node too?

Copy link
Member Author

@tompng tompng Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Fixed

lib/rdoc/parser/prism_ruby.rb Outdated
end

def _visit_call_public_private_class_method(call_node, visibility)
yield
Copy link
Member

@st0012 st0012 Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly, related, but why does this method call yield here but _visit_call_public_private_protected calls it in a branch?

Copy link
Member Author

@tompng tompng Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No specific reason. I think it's better to make it align.
If we're not calling yield (block is { super }) conditionally, calling super before calling this method is better.

I added this change:

_visit_call_public_private_class_method(args) { super }

def _visit_call_public_private_class_method(call_node, visibility)
  yield
  ...
end
# ↓
super
_visit_call_public_private_class_method(args)

def _visit_call_public_private_class_method(call_node, visibility)
  ...
end

@st0012 st0012 added the bug label Feb 8, 2026
tompng added 2 commits February 9, 2026 02:45
@tompng
Ignore visibility method, attr definition, module_function within block
a42b6f5 
We need to ignore these within `Module.new do end` and any other block because it might be a metaprogramming block.
@tompng
Simplify calling super in visit_call_node
96f88d1
@tompng tompng force-pushed the ignore_within_block branch from de00323 to 96f88d1 Compare February 8, 2026 17:47
@tompng tompng temporarily deployed to fork-preview-protection February 8, 2026 17:47 — with GitHub Actions Inactive
tompng
tompng commented Feb 8, 2026
View reviewed changes
lib/rdoc/parser/prism_ruby.rb
@in_proc_block = true
yield
@in_proc_block = false
@in_proc_block = in_proc_block
Copy link
Member Author

@tompng tompng Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

tap do
  include A # in_proc_block = true
  tap do end # Exiting this block was making `@in_proc_block = false
  include B # in_proc_block was false, fixed to true
end

@tompng tompng merged commit 7e1157a into ruby:master Feb 9, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@st0012 st0012 st0012 approved these changes

Assignees

No one assigned

Labels

bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tompng @matzbot @st0012